Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not show errors before user interaction #38

Merged
merged 2 commits into from
Nov 25, 2015

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Nov 20, 2015

Errors aren't shown before a user interaction. If user interacts with a field errors for that field are shown. If user submits the form, errors for all field of the form are shown.

What is treated as an user interaction differs by field. Only if an interaction could be seen as finished, it's causing errors to be shown. All focus out events are treated as finished user interactions. A change event on an input field isn't to prevent errors from popping up, while user inserts a valid string (e.g. if a minimum length is required). On the other hand an change on select and checkbox is.

This topic was discussed intensively in #15. There were many different ideas how it could be implemented. This PR tries to reflect the last state of the debate.

In opposite to feature/delayed-validations branch this is not optional. I don't think there are use cases where you want to show errors for untouched fields to the user. If there are use cases one could pass true to shouldShowErrors property of fields.
There shouldn't be any changes for people using server-side validations, since there these ones aren't present before a form is submitted.

jelhan added a commit to jelhan/croodle that referenced this pull request Nov 22, 2015
@g-cassie
Copy link
Collaborator

Agreed that this pretty much implements what was discussed in #15 and I think this makes sense.

Personally, I do not use fm-form currently so the manual override would be helpful if user does not touch a field at all and tries to proceed... Even if you are using fm-form sometimes there may be multiple buttons you can use to proceed (not just submit) so I think others may find it useful to override if necessary.

Errors aren't shown before a user interaction. If user interacts with a field
errors for that field are shown. If user submits the form, errors for all field
of the form are shown.

What is treated as an user interaction differs by field. Only if an interaction
could be seen as finished, it's causing errors to be shown. All focus out events
are treated as finished user interactions. A change event on an input field isn't
to prevent errors from popping up, while user inserts a valid string (e.g. if a
minimum length is required). On the other hand an change on select and checkbox
is.

This topic was discussed intensively in Emerson#15. There were many different ideas how
it could be implemented. This PR tries to reflect the last state of the debate.

In opposite to branch feature/delayed-validations this is not optional. I don't
think there are use cases where you want to show errors for untouched fields to
the user. If there are use cases one could pass `true` to `showErrors` property
of fields.
There shouldn't be any changes for people using server-side validations, since
there these ones aren't present before a form is submitted.
therefore hat to drop parentView access in fm-radio as already suggested here
Emerson#42 (comment)
@jelhan
Copy link
Contributor Author

jelhan commented Nov 25, 2015

Rebased after #41 and #42 were merged.

@g-cassie was right in suggesting to drop parentView in #42. Added another commit to this PR dropping some parentView access in fm-radio since it caused tests to fail after rebase.

@Emerson
Copy link
Owner

Emerson commented Nov 25, 2015

@jelhan I really appreciate all the work you're putting into this. Been wanting to get to this functionality for awhile now.

Emerson added a commit that referenced this pull request Nov 25, 2015
do not show errors before user interaction
@Emerson Emerson merged commit bda1b05 into Emerson:master Nov 25, 2015
@jelhan jelhan mentioned this pull request Jan 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants